Create new endpoint for expose style editor schemas#35377
Conversation
|
Claude finished @dario-daza's task in 1m 52s —— View job
PR Review1.
console.log('styleEditorSchemas', styleEditorSchemas);2. Schema fetch is now sequential — performance regression Previously both the GraphQL call and the schema fetch ran concurrently via 3. Silent JSON parse failure in
// Add onFailure logging:
Try.of(() -> {
final String schemaStr = (String) meta.get("DOT_STYLE_EDITOR_SCHEMA");
return (Object) mapper.readTree(schemaStr);
})
.onFailure(e -> Logger.warn(PageResourceHelper.class,
"Could not parse DOT_STYLE_EDITOR_SCHEMA: " + e.getMessage()))
.getOrNull()4. No null guard before
final String schemaStr = (String) meta.get("DOT_STYLE_EDITOR_SCHEMA");
if (schemaStr == null) return null;
return (Object) mapper.readTree(schemaStr);5. OpenAPI description still incorrect — unresolved
6. Leftover TODO comment
Clean
|
|
Claude finished @dario-daza's task in 2m 35s —— View job PR Review
Issues Found1. Permission level too high for a read operation
APILocator.getPermissionAPI().checkPermission(page, PermissionLevel.EDIT, user);
// Should likely be PermissionLevel.READ2.
3. Silent failure swallows JSON parse errors in
// Current: errors silently discarded
.map(meta -> Try.of(() -> {
final String schemaStr = (String) meta.get("DOT_STYLE_EDITOR_SCHEMA");
return (Object) mapper.readTree(schemaStr);
}).getOrNull())
// Should add onFailure:
.map(meta -> Try.of(() -> {
final String schemaStr = (String) meta.get("DOT_STYLE_EDITOR_SCHEMA");
return (Object) mapper.readTree(schemaStr);
})
.onFailure(e -> Logger.warn(PageResourceHelper.class, "Could not parse DOT_STYLE_EDITOR_SCHEMA: " + e.getMessage()))
.getOrNull())4. ClassCastException suppressed if schema value is not a String
5. OpenAPI description inconsistency
6. No unit or integration tests for new code Neither
7. Minor: Javadoc on
Clean
|
…type-tab-as-single-source-of-truth-for-headless-and-traditional-pages-frontend' into 35270-unify-style-editor-schema-definition-new-endpoint-for-schemas
| * @return Parsed schema objects, one per distinct content type that defines a schema; never | ||
| * {@code null}, may be empty. | ||
| */ | ||
| public static List<Object> getStyleEditorSchemas(final List<Contentlet> contentlets) { |
There was a problem hiding this comment.
Please remind me why we're pulling out here
| variantId != null ? variantId : VariantAPI.DEFAULT_VARIANT.name()); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
There's a great opportunity here for an integration test. To corroborate that the stuff that actually lives in the page gets pulled out with no dups, etc..
| }).getOrNull()) | ||
| .orElse(null)) | ||
| .filter(Objects::nonNull) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
I would add a test here also
Proposed Changes
GET /api/v1/page/{pageId}/contenttype-schemaThis PR fixes: #35270
This PR fixes: #35270